Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: allow to use env variables in config #2819

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: allow to use env variables in config #2819

wants to merge 2 commits into from
+92 −2

Conversation

ikatlinsky
Copy link

@ikatlinsky ikatlinsky commented Jun 5, 2023 •
edited
Loading

Why submit this pull request?

  • Bugfix
  • New feature provided
  • Improve performance
  • Backport patches

What changes will this PR take into?

This ticket adds an ability to use environment variables in the dashboard configuration using viper decode hooks. Such change is required to simplify dashboard configuration based on the environment type. For example can be useful to provide dashboard users and their passwords.

Related issues

#2818

Checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible?

@codecov-commenter
Copy link

codecov-commenter commented Jun 5, 2023 •
edited
Loading

Codecov Report

Merging #2819 (14c292b) into master (e0290c8) will decrease coverage by 2.84%.
The diff coverage is 87.50%.

❗ Current head 14c292b differs from pull request most recent head 06577e2. Consider uploading reports for the commit 06577e2 to get more accurate results

@@            Coverage Diff             @@
##           master    #2819      +/-   ##
==========================================
- Coverage   77.49%   74.65%   -2.84%     
==========================================
  Files         139      201      +62     
  Lines        3746     7860    +4114     
  Branches      881      881              
==========================================
+ Hits         2903     5868    +2965     
- Misses        843     1681     +838     
- Partials        0      311     +311
Flag Coverage Δ
backend-e2e-test-ginkgo 65.16% <87.50%> (?)
backend-unit-test 50.10% <87.50%> (?)
frontend-e2e-test 77.44% <ø> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
api/internal/conf/conf.go 67.87% <87.50%> (ø)

... and 62 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ikatlinsky
Copy link
Author

@Gallardot fixed the failing unit test, please approve workflow execution

Gallardot
Gallardot approved these changes Jun 6, 2023
View reviewed changes
Copy link
Member

@Gallardot Gallardot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ikatlinsky
Copy link
Author

@juzhiyuan @bzp2010 @guoqqqi Sorry guys, can we proceed with this feature? Your reviews are very desired, thanks.

@ikatlinsky
Copy link
Author

@juzhiyuan @bzp2010 @guoqqqi up for review, thanks

@juzhiyuan
Copy link
Member

Hello, I personally not available those months.

@ikatlinsky
Copy link
Author

@bzp2010 @Baoyuantop hi, hope you will be able to check proposed changes, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gallardot Gallardot Gallardot approved these changes

@juzhiyuan juzhiyuan Awaiting requested review from juzhiyuan

@bzp2010 bzp2010 Awaiting requested review from bzp2010

@guoqqqi guoqqqi Awaiting requested review from guoqqqi

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ikatlinsky @codecov-commenter @juzhiyuan @Gallardot @ikatlinski